Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix conditional #[wasm_bindgen] in impls #1208

Merged
merged 1 commit into from
Jan 28, 2019

Conversation

alexcrichton
Copy link
Contributor

Reported in #1191 the fix requires us to get a bit creative I think. The
general gist is that a block like this:

#[wasm_bindgen]
impl Foo {
    pub fn foo() {}
}

was previously expanded all in one go. Now, however, it's expanded into:

impl Foo {
    #[__wasm_bindgen_class_marker(Foo = "Foo")]
    pub fn foo() {}
}

// goop generated by orginal #[wasm_bindgen]

This method of expansion takes advantage of rustc's recursive expansion
feature. It also allows us to expand impl blocks and allow inner items
to not be fully expanded yet, such as still having #[cfg] attributes
(like in the original bug report).

We use theinternal __wasm_bindgen_class_marker to indicate that we're
parsing an ImplItemMethod unconditionally, and then generation
proceeds as usual. The only final catch is that when we're expanding in
an impl block we have to generate tokens for the Program
(wasm-bindgen injected goop like the custom section) inside the body
of the function itself instead of next to it. Otherwise we'd get syntax
errors inside of impl blocks!

Closes #1191

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with CI passing 👍

Reported in rustwasm#1191 the fix requires us to get a bit creative I think. The
general gist is that a block like this:

    #[wasm_bindgen]
    impl Foo {
        pub fn foo() {}
    }

was previously expanded all in one go. Now, however, it's expanded into:

    impl Foo {
        #[__wasm_bindgen_class_marker(Foo = "Foo")]
        pub fn foo() {}
    }

    // goop generated by orginal #[wasm_bindgen]

This method of expansion takes advantage of rustc's recursive expansion
feature. It also allows us to expand `impl` blocks and allow inner items
to not be fully expanded yet, such as still having `#[cfg]` attributes
(like in the original bug report).

We use theinternal `__wasm_bindgen_class_marker` to indicate that we're
parsing an `ImplItemMethod` unconditionally, and then generation
proceeds as usual. The only final catch is that when we're expanding in
an `impl` block we have to generate tokens for the `Program`
(wasm-bindgen injected goop like the custom section) inside the body
of the function itself instead of next to it. Otherwise we'd get syntax
errors inside of impl blocks!

Closes rustwasm#1191
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wasm_bindgen(constructor) doesn't compile with cfg_attr
2 participants